Skip to content

feat(flags): Implement codelyze flags#826

Draft
iangabrielsanchez wants to merge 4 commits into
mainfrom
ian/codelyze-flags
Draft

feat(flags): Implement codelyze flags#826
iangabrielsanchez wants to merge 4 commits into
mainfrom
ian/codelyze-flags

Conversation

@iangabrielsanchez

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for “flags” to the Codelyze coverage GitHub Action so teams can upload/report multiple coverage streams (e.g., unit vs e2e) independently, including per-flag commit statuses and a PR coverage summary comment.

Changes:

  • Introduces a new flag action input and passes it through to the backend coverage upload.
  • Posts per-flag commit statuses by fetching flag summaries from the backend.
  • Adds PR comment upsert logic to show overall + per-flag coverage in a single comment.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/main.ts Reads new flag input and forwards it into coverage processing.
src/coverage.ts Sends flag/parentShas to backend; adds per-flag statuses + PR comment upsert.
src/comment.ts New module to fetch flag summaries and upsert a PR comment with a marker.
src/codelyze.ts Extends upload payload to include flag and parentShas.
action.yml Documents and exposes the new flag input.
README.md Documents how to use flags and adds required permissions for PR comments.
.github/workflows/ci.yml Updates workflow permissions and example usage to include flag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/comment.ts Outdated
Comment on lines +69 to +72
// GITHUB_REF=refs/pull/123/merge or GITHUB_EVENT_PATH payload
const ref = process.env.GITHUB_REF ?? ''
const match = ref.match(/refs\/pull\/(\d+)\//)
return match ? Number(match[1]) : undefined

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says this supports reading the PR number from GITHUB_EVENT_PATH, but the implementation only parses GITHUB_REF. Either update the comment to match reality or implement the GITHUB_EVENT_PATH fallback to avoid confusion for future maintainers.

Copilot uses AI. Check for mistakes.
Comment thread src/comment.ts Outdated
Comment on lines +85 to +87
const params = new URLSearchParams({ token, commit, branch })
const res = await fetch(
`https://api.codelyze.com/v1/projects/coverage/flags?${params}`

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchFlagSummaries puts the codelyze upload token into the URL query string. Query strings are more likely to be captured in intermediary/proxy logs and error reports than headers or POST bodies. Prefer sending the token in an Authorization header (or request body) if the API supports it, and consider adding a request timeout (e.g., via AbortSignal.timeout) so the action can’t hang indefinitely on a stalled network call.

Suggested change
const params = new URLSearchParams({ token, commit, branch })
const res = await fetch(
`https://api.codelyze.com/v1/projects/coverage/flags?${params}`
const params = new URLSearchParams({ commit, branch })
const res = await fetch(
`https://api.codelyze.com/v1/projects/coverage/flags?${params}`,
{
headers: {
Authorization: `Bearer ${token}`
},
signal: AbortSignal.timeout(10000)
}

Copilot uses AI. Check for mistakes.
Comment thread src/comment.ts
Comment on lines +112 to +117
const rows = flags
.map((f) => {
const rate = f.linesFound > 0 ? f.linesHit / f.linesFound : 0
const cf = f.carryforward ? ' _(cf)_' : ''
return `| \`${f.flagName}\`${cf} | ${f.linesHit}/${f.linesFound} | ${percentString(rate)} |`
})

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flagName is interpolated directly into a Markdown table cell. If a flag name contains characters like | or backticks/newlines, it can break the table formatting and allows Markdown injection (e.g., unwanted mentions/links) in the PR comment. Consider escaping Markdown-sensitive characters (at least |, `, and newlines) before rendering, or validate/sanitize flag names to a safe character set.

Copilot uses AI. Check for mistakes.
Comment thread src/coverage.ts
Comment on lines +139 to +149
await upsertPrComment({
octokit,
context,
token,
commit: sha,
branch: ref?.replace('refs/heads/', '') ?? '',
overallHit: summary.lines.hit,
overallFound: summary.lines.found,
diff
}).catch((err) => core.debug(`PR comment failed: ${err}`))

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new block adds external API calls (fetchFlagSummaries) and PR comment upserts. The repo has Jest coverage for coverage.ts, but there are no tests asserting this new behavior (and the unmocked fetch/Octokit calls can introduce network dependency or unexpected throws during tests). Please add/update tests to mock the comment module and verify: (1) per-flag statuses are posted with the expected contexts, and (2) PR comment upsert is invoked only when appropriate.

Suggested change
await upsertPrComment({
octokit,
context,
token,
commit: sha,
branch: ref?.replace('refs/heads/', '') ?? '',
overallHit: summary.lines.hit,
overallFound: summary.lines.found,
diff
}).catch((err) => core.debug(`PR comment failed: ${err}`))
const isPullRequestEvent =
github.context.eventName === 'pull_request' ||
github.context.eventName === 'pull_request_target'
const hasPullRequest = Boolean(
(github.context.payload as { pull_request?: unknown }).pull_request
)
if (isPullRequestEvent && hasPullRequest) {
await upsertPrComment({
octokit,
context,
token,
commit: sha,
branch: ref?.replace('refs/heads/', '') ?? '',
overallHit: summary.lines.hit,
overallFound: summary.lines.found,
diff
}).catch((err) => core.debug(`PR comment failed: ${err}`))
}

Copilot uses AI. Check for mistakes.
Comment thread src/main.ts Outdated
const patchThreshold =
Number.parseFloat(core.getInput('patch-threshold')) || 0
const emptyPatch = core.getBooleanInput('skip-empty-patch') ?? false
const flag = core.getInput('flag') || undefined

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flag is read without trimming whitespace. Since this value becomes part of the upload payload/status context, leading/trailing spaces can easily produce unexpected “different” flags. Consider using core.getInput('flag', { trimWhitespace: true }) and/or normalizing to a safe identifier format before passing it through.

Copilot uses AI. Check for mistakes.
Comment thread src/comment.ts Outdated
Comment on lines +43 to +49
const comments = await octokit.rest.issues.listComments({
owner: context.owner,
repo: context.repo,
issue_number: prNumber
})

const existing = comments.data.find((c) => c.body?.includes(MARKER))

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listComments only returns the first page (30 comments by default). On PRs with lots of discussion, the existing codelyze marker comment may be on a later page, causing this to create duplicate comments instead of updating. Consider using octokit.paginate (or explicitly paginating with per_page: 100 and page iteration) when searching for the marker comment.

Suggested change
const comments = await octokit.rest.issues.listComments({
owner: context.owner,
repo: context.repo,
issue_number: prNumber
})
const existing = comments.data.find((c) => c.body?.includes(MARKER))
const comments = await octokit.paginate(octokit.rest.issues.listComments, {
owner: context.owner,
repo: context.repo,
issue_number: prNumber,
per_page: 100
})
const existing = comments.find((c) => c.body?.includes(MARKER))

Copilot uses AI. Check for mistakes.
@github-actions

github-actions Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Coverage report for f1ebe7c

Lines Coverage
Overall 117/183 63.93%

cf = carried forward from a previous commit

@megheaiulian megheaiulian marked this pull request as draft May 9, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants